Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removing builder-build from build and adding instructions how to build the builder #5

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

MalteJ
Copy link
Member

@MalteJ MalteJ commented Jun 17, 2023

Fixes #4

@gardener-robot
Copy link
Contributor

@MalteJ Thank you for your contribution.

@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Jun 17, 2023
@@ -58,11 +58,6 @@ container_mount_opts=(
-v "$PWD/$target_dir:/builder/.build"
)

if [ "$container_image" = localhost/builder ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the build script is intended to speed up local dev / testing, by allowing to automatically rebuild the builder if any local changes were made.
It works if instead of downloading a release build script and placing it in the config directory, you instead sym link to the script inside a local copy of the builder repo.
We should not remove this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably document how this local dev / testing setup works though 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but how do I use a locally built builder in a separate repo, that holds my features?
When I set the container image in the other repo to localhost/builder it tries to build the builder. Instead, it should use the builder that I have built before using docker build -t localhost/builder .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to build the builder in the same repo that hold my features. I want to have it separated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to use a symlink to the build script inside the builder repo instead of a copy of the build script when doing local dev on the builder. That way the builder and the features are still separated, but the builder automatically detects that it is running in local dev mode and rebuilds itself iff necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. Can you plz provide an example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this rename necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker wants to have a Dockerfile. Podman can work with both.

@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Jun 19, 2023
Copy link
Contributor

@Vincinator Vincinator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to me

@Vincinator Vincinator merged commit 8cbfb09 into main Jun 21, 2023
2 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Jun 21, 2023
@MalteJ MalteJ deleted the feat/contribute branch June 21, 2023 12:49
@nkraetzschmar nkraetzschmar mentioned this pull request Jun 26, 2023
nkraetzschmar added a commit that referenced this pull request Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/changes Needs (more) changes needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide Developer Docs
4 participants